-
Notifications
You must be signed in to change notification settings - Fork 400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement ENTSO-E, Time-of-Use Tariff provider #2207
Conversation
Implemented new Exchange rate API.
Modifed the provider to activate when currency is updated.
Refactored code. Modified the test cases.
...nems.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/BiddingZone.java
Outdated
Show resolved
Hide resolved
...ms.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/TouEntsoeImpl.java
Outdated
Show resolved
Hide resolved
...ms.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/TouEntsoeImpl.java
Outdated
Show resolved
Hide resolved
io.openems.edge.core/src/io/openems/edge/core/meta/MetaImpl.java
Outdated
Show resolved
Hide resolved
...ms.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/TouEntsoeImpl.java
Outdated
Show resolved
Hide resolved
...ms.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/TouEntsoeImpl.java
Outdated
Show resolved
Hide resolved
...ms.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/TouEntsoeImpl.java
Show resolved
Hide resolved
...ms.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/TouEntsoeImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A first review
io.openems.edge.common/src/io/openems/edge/common/currency/Currency.java
Outdated
Show resolved
Hide resolved
io.openems.edge.timeofusetariff.entsoe/.settings/org.eclipse.core.resources.prefs
Show resolved
Hide resolved
...nems.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/BiddingZone.java
Outdated
Show resolved
Hide resolved
...edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/CurrencyProvider.java
Outdated
Show resolved
Hide resolved
....edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/ExchangeRateApi.java
Show resolved
Hide resolved
...ms.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/TouEntsoeImpl.java
Outdated
Show resolved
Hide resolved
....edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/ExchangeRateApi.java
Outdated
Show resolved
Hide resolved
....timeofusetariff.entsoe/test/io/openems/edge/timeofusetariff/entsoe/ExchangeRateApiTest.java
Outdated
Show resolved
Hide resolved
Added Dummy Component. Added Currency config.
@venu-sagar Should I review again already? Or will you announce me? |
I will announce you. |
1 similar comment
@sfeilmeier You can review it now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comments.
Unfortunately I am way to much involved again in coding style and coding details. Please have somebody else review it next time before asking for a final review. Thanks!
io.openems.edge.core/src/io/openems/edge/core/meta/MetaImpl.java
Outdated
Show resolved
Hide resolved
...nems.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/BiddingZone.java
Outdated
Show resolved
Hide resolved
io.openems.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/Config.java
Outdated
Show resolved
Hide resolved
* | ||
* @param updateCurrency The callback {@link Consumer}. | ||
*/ | ||
private void subscribe(Consumer<Currency> updateCurrency) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All this subscribe and unsubscribe is ok now, but it's still quite over engineered.
It would be sufficient to keep a private final variable:
private final Consumer<Currency> updateCurrency = currency -> {
this.currency = currency;
this.executor.schedule(this.task, 0, TimeUnit.SECONDS);
};
then subscribe during activate()
(or setMeta()
) and unsubscribe during deactivate()
(or unsetMeta()
). No need to keep a List of subscribes when you know you have only one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You got my comment wrong. With setMeta()
I referred to using "Method Injection" (see OSGi compendium 112.3.2 Method Injection https://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.component.html#service.component-method.injection)
...ms.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/TouEntsoeImpl.java
Outdated
Show resolved
Hide resolved
...ms.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/TouEntsoeImpl.java
Outdated
Show resolved
Hide resolved
....timeofusetariff.entsoe/test/io/openems/edge/timeofusetariff/entsoe/ExchangeRateApiTest.java
Outdated
Show resolved
Hide resolved
...s.edge.timeofusetariff.entsoe/test/io/openems/edge/timeofusetariff/entsoe/TouEntsoeTest.java
Outdated
Show resolved
Hide resolved
& some small improvements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments and implemented directly: dd37ceb
Please test again; afterwards from my side we are ready to merge
@@ -2,9 +2,9 @@ | |||
|
|||
This implementation uses the ENTSO-E transparency platform to receive day-ahead prices in European power grids. | |||
|
|||
To request a (free) authentication token, please see chapter "2. Authentication and Authorisation" in the official API documentation: https://transparency.entsoe.eu/content/static_content/Static%20content/web%20api/Guide.html#_authentication_and_authorisation | |||
To request a (free) authentication token, please see chapter "2. Authentication and Authorization" in the official API documentation: https://transparency.entsoe.eu/content/static_content/Static%20content/web%20api/Guide.html#_authentication_and_authorisation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the document it is actually written in British English "Authorisation" and not "Authorisation"
* | ||
* @param updateCurrency The callback {@link Consumer}. | ||
*/ | ||
private void subscribe(Consumer<Currency> updateCurrency) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You got my comment wrong. With setMeta()
I referred to using "Method Injection" (see OSGi compendium 112.3.2 Method Injection https://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.component.html#service.component-method.injection)
...ms.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/TouEntsoeImpl.java
Outdated
Show resolved
Hide resolved
...ms.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/TouEntsoeImpl.java
Outdated
Show resolved
Hide resolved
...ms.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/TouEntsoeImpl.java
Outdated
Show resolved
Hide resolved
...ms.edge.timeofusetariff.entsoe/src/io/openems/edge/timeofusetariff/entsoe/TouEntsoeImpl.java
Outdated
Show resolved
Hide resolved
I have tested it. Working as expected. |
Well done. Thank you @venu-sagar! |
This implementation uses the ENTSO-E transparency platform to receive day-ahead prices in European power grids.
See https://newtransparency.entsoe.eu/dv/market/price/dayAhead/PT60M
To request a (free) authentication token, please see chapter "2. Authentication and Authorisation" in the official API documentation: https://transparency.entsoe.eu/content/static_content/Static%20content/web%20api/Guide.html#_authentication_and_authorisation